fix(host): correct schema name in new data modules, sync init.sql, update tests - #47
Open
TalkingJupiter wants to merge 1 commit into
Open
fix(host): correct schema name in new data modules, sync init.sql, update tests#47TalkingJupiter wants to merge 1 commit into
TalkingJupiter wants to merge 1 commit into
Conversation
…, update tests
current_state.get_recent and unknown_devices.record both queried
repacss_enviroment, which is not a schema under either spelling. Both wrap
their SQL in a log-and-continue except, so they failed silently: get_recent
always returned {} and every unknown device record was dropped with only a
printed error.
init.sql still described unknown_devices as (first_seen, last_seen,
message_count_24h) while the deployed table and the module use hit_count
plus last_payload. Bring init.sql in line with the database rather than
changing the code, and note that hit_count is a running total: nothing
trims it, so the 24 hour summary in the README is a query over hit_count
and last_seen, not a maintained counter.
Update the provisioning tests for the data-module refactor: is_enabled,
get_device_state and upsert_device_state are gone or moved, build_config
now takes enabled straight from the registry row, and an unknown device is
recorded and deliberately not answered so it keeps helloing. Add tests for
current_state and unknown_devices, including guards for the schema name and
for keeping an untrusted mac in the parameters rather than the SQL text.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the per-table data module refactor (
2208831). Two of the new modules could never have worked,init.sqlhad drifted from the deployed schema, and the provisioning tests still targeted the pre-refactor API.Silent failures in the new modules
current_state.get_recentandunknown_devices.recordboth queriedrepacss_enviroment(missing then), which is not a schema under either spelling. Both wrap their SQL in a log-and-continueexcept, so they failed silently:get_recentalways returned{}, and every unknown-device record was dropped with only a printed error.device_registryandingestion_servicespell it correctly, so only the two new modules were affected.init.sql was the stale side
It still described
unknown_devicesas(first_seen, last_seen, message_count_24h)while the deployed table and the module usehit_countpluslast_payload. Updatedinit.sqlto match the database rather than changing the code.Also documented in the schema that
hit_countis a running total, not a rolling window: nothing trims it, so the "24 h count" the README describes is a query overhit_countandlast_seen, not a maintained counter. That distinction was implied by the old column name and is now explicit.Tests
The refactor removed
is_enabled,get_device_stateandupsert_device_statefromprovisioning_service, leaving 13 tests failing against a non-existent API. Rewritten:build_confignow takesenabledstraight from the registry row (the role filter is gone sincedevice_map.enabledis the truth), plus a case forenabled:falsepassing through as an applied config.MAX_HELLO_BYTESoversized-payload drop.test_current_state.pyandtest_unknown_devices.pyfor the extracted modules.Two of the new tests are regression guards for exactly what went wrong here: one asserts
get_recentnamesrepacss_environment.current_status, so the typo cannot come back silently; one passes a SQL-injection string as the MAC and asserts it stays in the parameters tuple rather than the SQL text.43 tests pass. No behavior changes beyond the two corrected schema names.
Not addressed
ingestion_service.handle_unknown_device()is still a no-op, so unknown telemetry is dropped silently while provisioning now records unknown hellos. Wiring it tounknown_devices.recordneeds rate-limiting first, since telemetry arrives every 5 s.